Skip to content

Comments

fix: location claim processing#286

Merged
fforbeck merged 6 commits intomainfrom
fix/location-claim-processing
Jan 19, 2026
Merged

fix: location claim processing#286
fforbeck merged 6 commits intomainfrom
fix/location-claim-processing

Conversation

@fforbeck
Copy link
Member

When querying IPNI for location claims, the indexing service now iterates through all provider results instead of failing on the first error. This allows queries to succeed as long as at least one valid provider result exists, even if others have incomplete or invalid addresses.

See this INPI record as an example: https://cid.contact/cid/bagbaieraqcpyzx3sytzsitbdxqpahht3hw4ros5ex2jlhxe4muwcgc7d4xua

ProviderResults[1] is invalid, has only 1 address, and it is missing the location URL.

ProviderResults[0] is correct, but the indexer never finds this record.

This fixes the issue where IPNI records with missing blob retrieval endpoints (e.g., only claim URL, no blob URL) would cause the entire query to fail with a 500 error, even when other valid provider results were available.

The implementation follows the same pattern used for index claim processing, where all results are tried, and only the last error is returned if all attempts fail.

@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
pkg/service/service.go 81.65% <100.00%> (+2.29%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@volmedo volmedo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I have a general question that might not make a lot of sense in this case: are there any errors that should be treated the old way? I.e. can any of the operations on a given result fail in a way that we should be failing the whole job (as we were doing before)? I guess the answer is no, and this approach works, but maybe worth doing a quick check of the possible error conditions to confirm. If there is any specific error that should be handled differently, we can create a sentinel for that and check.

@fforbeck fforbeck self-assigned this Jan 16, 2026
@alanshaw
Copy link
Member

I have a general question that might not make a lot of sense in this case: are there any errors that should be treated the old way? I.e. can any of the operations on a given result fail in a way that we should be failing the whole job (as we were doing before)? I guess the answer is no, and this approach works, but maybe worth doing a quick check of the possible error conditions to confirm. If there is any specific error that should be handled differently, we can create a sentinel for that and check.

I reckon no, we should always return a result with whatever we find. The result should contain the error messages. We should however always log errors and there are certain errors that should be reported e.g. we cannot talk to redis.

@fforbeck fforbeck force-pushed the fix/location-claim-processing branch from 09a273d to 9d02135 Compare January 19, 2026 14:49
@fforbeck fforbeck merged commit 9fa70e3 into main Jan 19, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants